fix(security): detect letter-spaced P3 and P4 prompts by mohgupta-ship-it · Pull Request #470 · NVIDIA/SkillSpector

mohgupta-ship-it · 2026-09-01T07:20:10Z

Summary

Follow-up to merged #408. This closes the remaining static-only bypass where a raw P3/P4 phrase is detected, but a fully or partly letter-spaced equivalent can return no findings and SAFE.

Reconstructs explicit letter-spaced tokens for P3/P4 only, with exact source-offset mapping.
Detects mixed forms such as s e n d conversation to external and never warn the u s e r.
Preserves punctuation inside reconstructed tokens, including URLs and possessives.
Keeps boundary-free reconstruction conservative: it emits AE6 + partial/CAUTION without inventing a P3/P4 classification.
Treats identifier-adjacent and oversized ambiguous runs as AE6-only rather than silently safe.
Works with --no-llm; MCP returns safe_to_install=false for covered attacks.

Algorithm

The projection uses a deterministic linear scanner rather than a large backtracking regex. It removes only observed single ASCII-space token interiors, preserves explicit multi-space/newline boundaries, and maintains a derived-to-raw offset map. Semantic matching remains scoped to the existing P3/P4 grammars.

flowchart TD
    A[Raw artifact text] --> B[Linear letter-spacing scanner]
    B --> C{Explicit boundaries reconstruct P3/P4?}
    C -->|Yes| D[P3 or P4 finding]
    D --> E[AE6 + partial analysis]
    C -->|No| F{Boundary-free or identifier-adjacent security grammar?}
    F -->|Yes| G[AE6 only; no guessed semantic rule]
    F -->|Oversized ambiguity| G
    F -->|No| H[Keep established result]
    E --> I[CAUTION / unsafe to install]
    G --> I

Regression coverage

Fully spaced, mixed-token, URL, apostrophe, identifier-adjacent, and all P3/P4 ambiguous families.
Benign spelling/initialism controls, including the always use rover/cover re-segmentation case.
Duplicate suppression across raw/normalized/compact views.
LF/CRLF source locations, cross-window ownership, long matches beyond overlap, and cooperative runtime checks.
Graph, CLI --no-llm, and MCP public surfaces.

Validation

552 passed across the affected security, end-to-end, artifact-integrity bounds, and static-pattern suites.
Ruff, formatting, mypy, and git diff --check passed.
Three-agent adversarial release council: security, code-quality, and public-surface test judges all voted RELEASE on the frozen diff.

Powered by Codex.

mohgupta-ship-it · 2026-09-01T08:36:25Z

CI follow-up pushed in 60fdd2a.

Root cause: NEW_REGRESSION / runtime-budget exhaustion. The letter-spacing projection walked every large plain-text window in Python even when no spacing candidate existed. Under pytest-cov overhead, static_patterns_prompt_injection crossed its 30-second artifact budget, became partial, and omitted the final P1 window.

Fix:

Fast-reject plain text with a fixed-width C-regex necessary-condition prefilter.
Avoid rescanning an identical identifier-relaxed projection.
Add an oversized plain-text regression proving the Python projection loop is skipped.

Validation:

CI-failing tests: passed with coverage.
Full security end-to-end file: 74 passed with coverage.
Affected suites: 553 passed.
Ruff, mypy, and diff checks: passed.

Powered by Codex.

yashrajp22

I spent a while on this one because the idea is good and the core machinery holds up: I verified end to end with --no-llm that the fully-spaced and double-space-mixed forms now get caught, offset mapping back to raw lines is correct, raw-vs-projected dedup works, and I couldn't find any ReDoS in the projection (1–4 MB adversarial inputs stay linear; the prefilter does its job).

But there's one gap I think is worth fixing before merge: the single-space mixed form of this PR's own headline attack strings still scans completely SAFE — details on the run scanner inline. There's also a reproducible false positive where benign snake_case code gets flagged as obfuscation. Both look fixable within the existing design.

One smaller thing that may just need documenting: a long run of benign spaced letters (think A C G T A C G T ... in docs or tables — I tried 600 chars) goes to AE6 HIGH + CAUTION + safe_to_install=false by design. Fail-closed is the right instinct, but that's a real documentation pattern, so worth a note or an allowlist path.

水位修正（tools/upstream_baseline.json）： reviewed_pr_through 483 → 462、reviewed_issue_through 482 → 0。原本那組數字等於宣稱「上游 PR 與 issue 都審過了」，但沒有人看過那 8 個仍開啟的 PR，一個上游 issue 也還沒對本 fork 分診過。462 是誠實的：本 fork 的 HEAD 就是 PR NVIDIA#462 的合併點，合併到 NVIDIA#462 為止的每個 PR 都字面存在於這棵樹裡，不需要移植；NVIDIA#462 以上的都是未合併、未審。issue 那一軸據實寫 0。判定記錄（docs/UPSTREAM.md，新增）： fork 繼承的 36 個分支全部給出書面判定——「刪掉」不等於「處理過」，判定要寫下來才算。分四組： - A（22 個）：commits 已 patch-id 相同地在 main 裡，內容已在樹上。 - B（4 個）：上游 PR 已定案。NVIDIA#332／NVIDIA#306 已合併＝已在樹上；NVIDIA#155／NVIDIA#235 關閉未合併，由同期的 -2 後續分支取代（此為依命名慣例與關閉時間的推論，檔內已標明不是上游明說）。 - C（8 個）：上游 PR 仍開啟，逐筆四點評估（缺陷是什麼／本樹是否有這段程式／判定／回頭再看的觸發點）。本樹與上游逐字元相同，所以這些缺陷在這裡全部存在——問題是「現在移植」還是「等上游合併」，不是「適不適用」。八筆全部判「等上游合併」，但各有各的理由與觸發點：NVIDIA#470 的 letter-spaced P3/P4 是純靜態路徑就能繞過的真實安全缺口，優先序最高；NVIDIA#383/NVIDIA#430/NVIDIA#442 是同一個 dependency-source redirection 能力的三個疊加嘗試，提前選邊會造成合併衝突白工。 - D（2 個）：從未成為 PR，任何水位都追不到。已從上游 fetch 回來評估後判「不適用」（NIM provider 是功能擴充非缺陷修正、且無使用情境；revert-306 是上游自己開了又放棄的提案），判定寫入後才刪除。驗證：pwsh -NoProfile -File tools/dev_check.ps1 → WINDOWS DEV CHECK GREEN、exit 0 （3953 passed、39 skipped、4 xfailed）。python 驗過 baseline JSON 可解析。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

rng1995

[SkillSpector Review]

Reviewed current head 70e0a150173ab28f986aa7a0bc9e80b7ff872ba3. Two correctness/security issues still require changes:

In src/skillspector/artifacts.py:1760, the spaced-run scanner consumes the first character of an immediately following word. For example, s e n d conversation to external becomes a run ending in the c from conversation; the right-boundary check then rejects it, and the advancement at line 1791 skips the whole run. Stop the run at the token boundary or retry valid subspans, and add this adjacent-word regression.
In src/skillspector/nodes/analyzers/artifact_integrity.py:660, the relaxed projection globally replaces digits and underscores after any collapsible spaced pair appears anywhere. A benign U S A can therefore arm the relaxation and make an unrelated ordinary identifier such as upload_files_to_external_service(config) look like an AE6 phrase. Require every relaxed match to map to and overlap a reconstructed spaced span, and add a regression with unrelated spaced text plus an underscore identifier.

The refreshed bot-merge head has no hosted check results yet and is currently blocked. It is not merge-ready.

rng1995

[SkillSpector Review]

Re-reviewed current head 2df5f91230dffcba6b926be94eb0722ffcfb3046. The later commits only synchronize main; all five PR-owned blobs are unchanged from the previously reviewed head.

Both blocking findings remain: the spacing scanner still consumes the first character of an immediately adjacent word and advances past the rejected run, so s e n d conversation to external and the analogous P4 form evade detection; and the artifact-integrity rescan still replaces digits and underscores across the whole projected document without requiring a match to overlap a reconstructed span, so unrelated spaced text can turn a benign identifier into AE6. The existing inline threads contain the exact cases and fixes, so I have not duplicated them. The two other open threads are nonblocking maintainability and performance notes.

No hosted checks are reported, and GitHub reports mergeStateStatus=BLOCKED.

rng1995

[SkillSpector Review]

Re-reviewed current head dec50548fc84d2e1930388bed6bd70b554bb6e9d. The commits since the marked review at 2df5f91230dffcba6b926be94eb0722ffcfb3046 only merge current main (including the configurable static-analysis allowance); all five PR-owned blobs are byte-for-byte unchanged.

Both blocking findings remain: the spacing scanner consumes the first character of an immediately adjacent word and advances past the rejected run, so s e n d conversation to external and the analogous P4 form evade detection; and artifact-integrity still replaces digits and underscores across the entire projected document without requiring the match to overlap a reconstructed span, so unrelated spaced text can turn a benign identifier into AE6. The existing inline threads contain the exact cases and fixes, so I have not duplicated them. The two other open threads remain nonblocking maintainability/performance notes.

No hosted checks are reported, and GitHub reports mergeStateStatus=BLOCKED.

rng1995

[SkillSpector Review]

Re-reviewed current head 2c48d1e4afbfb7f036f48a647041547cb0066a27 after the marked review at dec50548fc84d2e1930388bed6bd70b554bb6e9d. All five PR-owned blobs are byte-for-byte unchanged.

Both blocking findings remain. The spacing scanner still consumes the first character of an adjacent word and skips the rejected run, so s e n d conversation to external and the analogous P4 form evade detection. Artifact-integrity also relaxes digits and underscores across the entire projected document without requiring the match to overlap reconstructed text, allowing unrelated spaced prose to turn a benign identifier into AE6. The existing current threads contain the reproductions and expected fixes.

No required checks are reported, active change requests and unresolved threads remain, and GitHub reports mergeStateStatus=BLOCKED.

Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

rng1995 · 2026-09-14T21:15:25Z

Review follow-up summary for 4cf25bf (rebased cleanly onto main at 03e8f56):

The long spaced-sequence carve-out is intentionally narrow: only normalized A/C/G/T/N/U DNA/RNA alphabets are treated as benign. A regression appends a condensed prompt attack to a 600-base sequence and proves AE6 still fails closed.
Runtime cooperation enters through the shared static-runner budget callback and remains optional for direct analyzer calls. Deadline expiry preserves already-created findings, emits a PARTIAL/RUNTIME_LIMIT ledger event, and keeps the final recommendation at CAUTION.
Canonical unbounded P3/P4 relations remain intact, avoiding a silent 512/513-character detection boundary. Regexes are compiled once, plain-text projection fast-rejects in C, and Unicode-tag safe-span checking is now linear.
Local locked-environment validation: 333 remediation, 86 security end-to-end, 682 pattern, 607 reconstruction/deadline, and 17 budget tests passed; Ruff, formatting, targeted mypy, and diff checks passed.
Hosted CI is fully green: changes, lint, test-unit, DCO, and Docker smoke.

rng1995

[SkillSpector Review]

Re-reviewed current head 4cf25bf20fdaf0c24bceef527967610b7102f53a. Both findings from the prior marked review are resolved: adjacent ordinary words no longer get consumed into the spaced run, and underscore/digit relaxation is now restricted using exact reconstruction provenance. The prior review threads are resolved, the branch is current with main, and all hosted checks pass.

One fail-open case still requires correction. Alternating whitespace widths inside a short obfuscated word—for example, s e n d conversation to external—split the projection into fragments. P3/P4 do not match, and the concealed-run fallback remains below its six-letter threshold, so AE6 also does not fire and the skill can remain SAFE. The inline comment contains the reproduction and expected fail-closed behavior.

Because this affects the customer-requested letter-spacing bypass, this head is not merge-ready.

rng1995 · 2026-09-14T22:24:27Z

+                break
+
+            if whitespace_gap:
+                if spacing_width is not None and len(gap) != spacing_width:


[P1] Keep irregular spacing fail-closed. The fixed-width comparison splits a short obfuscated action when an attacker alternates one- and two-character gaps. For example, s e n d conversation to external becomes se nd conversation to external, so neither the raw nor projected P3 grammar matches. The concealed-run fallback sees only send plus the c lookahead—five letters, below its six-letter threshold—so AE6 also does not fire and the scan can remain SAFE. Preserve explicit word boundaries for semantic classification, but ensure inconsistent-width short runs are at least reported as AE6/incomplete. Please add end-to-end P3 and P4 regressions for alternating gap widths.

mohgupta-ship-it requested review from Spectorian, keshprad, rng1995 and yashrajp22 September 1, 2026 07:20

yashrajp22 reviewed Sep 2, 2026

View reviewed changes

rng1995 requested changes Sep 12, 2026

View reviewed changes

rng1995 requested changes Sep 14, 2026

View reviewed changes

mohgupta-ship-it and others added 3 commits September 14, 2026 13:57

fix(security): detect letter-spaced P3 and P4 prompts

bde2e21

Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>

fix(ci): fast-reject plain prompt projections

63b2625

Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>

fix(security): harden spaced prompt reconstruction

4cf25bf

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

rng1995 force-pushed the codex/p3-p4-no-llm-followup branch from ab27d26 to 4cf25bf Compare September 14, 2026 20:58

rng1995 requested review from rng1995 and yashrajp22 September 14, 2026 21:37

rng1995 requested changes Sep 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(security): detect letter-spaced P3 and P4 prompts - #470

fix(security): detect letter-spaced P3 and P4 prompts#470
mohgupta-ship-it wants to merge 3 commits into
mainfrom
codex/p3-p4-no-llm-followup

mohgupta-ship-it commented Sep 1, 2026

Uh oh!

mohgupta-ship-it commented Sep 1, 2026

Uh oh!

yashrajp22 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 commented Sep 14, 2026

Uh oh!

rng1995 left a comment

Uh oh!

rng1995 Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mohgupta-ship-it commented Sep 1, 2026

Summary

Algorithm

Regression coverage

Validation

Uh oh!

mohgupta-ship-it commented Sep 1, 2026

Uh oh!

yashrajp22 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

rng1995 commented Sep 14, 2026

Uh oh!

rng1995 left a comment

Choose a reason for hiding this comment

Uh oh!

rng1995 Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants